HAZEI Onboarding #1000

hazei-odoo · 2025-10-20T11:51:48Z

No description provided.

robodoo · 2025-10-20T11:51:50Z

Mathilde411

Small things here :)

Mathilde411 · 2025-10-20T14:18:16Z

estate/__init__.py

@@ -0,0 +1 @@
+from . import models


End of file line is missing here

Mathilde411 · 2025-10-20T14:18:41Z

estate/__manifest__.py

+    'summary' : 'Real Estate Management Tutorial',
+    'application': True,
+
+}


End of file line is missing here

Mathilde411

Nice work here ! :D
I didn't highlight them all but be careful about the naming of the ids/files/names according to: https://www.odoo.com/documentation/19.0/contributing/development/coding_guidelines.html

Mathilde411 · 2025-10-22T07:46:56Z

estate/models/estate_property.py

+    name = fields.Char(string="Title", required=True)
+    description = fields.Text()
+    postcode = fields.Char()
+    date_availability = fields.Date(copy=False, readonly=True, default=date.today() + relativedelta(months=3))


Suggested change

date_availability = fields.Date(copy=False, readonly=True, default=date.today() + relativedelta(months=3))

date_availability = fields.Date(copy=False, readonly=True, default=lambda self: date.today() + relativedelta(months=3))

If you don't do this, it will only be evaluated one time and not work tomorrow on a long running instance.

Mathilde411 · 2025-10-22T07:47:48Z

estate/models/estate_property.py

+        string='garden_orientation',
+        selection=[('north', 'North'), ('south', 'South'),
+        ('east', 'East'), ('west', 'West')])
+    active = fields.Boolean(default=False)


Suggested change

active = fields.Boolean(default=False)

active = fields.Boolean()

False is already the default for Boolean

alan-odoo

Hi @hazei-odoo 👋

As you asked, I checked your development. My comments are mainly about python conventions 👋.

Also do not forget to add a title and a description to your PR. The title must have the same structure than a commit title -> [ADD/IMP/FIX...] app: the purpose of the pr (must start by a verb). And for the description, it must mainly explain why the pr is doing this and not how.

Thanks for you work 😃!

estate/models/__init__.py

alan-odoo · 2025-10-23T07:12:36Z

estate/models/estate_property.py

+from odoo import models, fields, api
+from datetime import date
+from dateutil.relativedelta import relativedelta
+from odoo.exceptions import UserError


The imports should also be sorted alphabetically. I would add that external imports (all imports “date...” for example) must be imported first, followed by internal imports (starting with “from odoo”).

alan-odoo · 2025-10-23T07:19:27Z

estate/models/estate_property.py

+    _description = "Real Estate Property"
+
+    name = fields.Char(string="Title", required=True)
+    Property_Type = fields.Text()


Should be property_type. Variables use only lowercase letters. Uppercase letters are used by constants and classes.

Variable -> this_is_a_variable =
constant -> THIS_IS_A_CONSTANT
class -> ThisIsAClass

alan-odoo · 2025-10-23T07:42:16Z

estate/models/estate_property.py

+    )
+
+    def action_set_sold(self):
+        if (self.state != "Cancelled"):


No need of the parenthesis. The != evaluate if the two elements are similar and will return a boolean value in any case.

Suggested change

if (self.state != "Cancelled"):

if self.state != "Cancelled":

estate/models/estate_property.py

estate/views/estate_property_type.xml

estate/views/menu_views.xml

estate/views/estate_property_type.xml

estate/views/estate_property_views.xml

Mathilde411

Good job !
Not much to say. Did like I would on a real review.

Mathilde411 · 2025-10-24T11:09:38Z

estate/models/estate_property.py

+    selection=[
+        ('new', 'New'),
+        ('offer_received', 'Offer Received'),
+        ('offer_accepted', 'Offer Accepted'),
+        ('sold', 'Sold'),
+        ('cancelled', 'Cancelled')]
+    )


This is a constant, so it should go outside the class and be given a more consistent name like PROPERTY_STATES

Mathilde411 · 2025-10-24T11:10:33Z

estate/models/estate_property.py

+    def action_set_sold(self):
+        if self.state != "cancelled":
+            self.state = "sold"
+        else:
+            raise UserError("A cancelled property can not be sold")
+        return True


Suggested change

def action_set_sold(self):

if self.state != "cancelled":

self.state = "sold"

else:

raise UserError("A cancelled property can not be sold")

return True

def action_set_sold(self):

if self.state == "cancelled":

raise UserError("A cancelled property can not be sold")

self.state = "sold"

return True

More concise and extendable

Also what would happen here if self contains more than 1 property ?

Mathilde411 · 2025-10-24T11:12:00Z

estate/models/estate_property.py

+    def action_set_sold(self):
+        if self.state != "cancelled":
+            self.state = "sold"
+        else:
+            raise UserError("A cancelled property can not be sold")
+        return True
+
+    def action_set_cancelled(self):
+        if (self.state != "sold"):
+            self.state = "cancelled"
+        else:
+            raise UserError("A sold property can not be cancelled")
+        return True


As per attribute ordering (coding guidelines), this should go lower in the file

Mathilde411 · 2025-10-24T11:12:46Z

estate/models/estate_property.py

+    def action_set_cancelled(self):
+        if (self.state != "sold"):
+            self.state = "cancelled"
+        else:
+            raise UserError("A sold property can not be cancelled")
+        return True


Suggested change

def action_set_cancelled(self):

if (self.state != "sold"):

self.state = "cancelled"

else:

raise UserError("A sold property can not be cancelled")

return True

def action_set_cancelled(self):

if self.state == "sold":

raise UserError("A sold property can not be cancelled")

self.state = "cancelled"

return True

No parentheses around if, and more concise

Also what would happen here if self contains more than 1 property ?

Mathilde411 · 2025-10-24T11:15:58Z

estate/models/estate_property.py

+    @api.depends('living_area', 'garden_area')
+    def _compute_total_area(self):
+        for property in self:
+            property.total_area = property.living_area + (property.garden_area or 0)


Suggested change

property.total_area = property.living_area + (property.garden_area or 0)

property.total_area = property.living_area + property.garden_area

garden_area is an integer, it cannot be False

Mathilde411 · 2025-10-24T11:16:42Z

estate/models/estate_property.py

+    def _compute_best_price(self):
+        for record in self:
+            if record.offer_ids:
+                record.best_price = max(record.offer_ids.mapped('price')) if record.offer_ids else 0


Suggested change

def _compute_best_price(self):

for record in self:

if record.offer_ids:

record.best_price = max(record.offer_ids.mapped('price')) if record.offer_ids else 0

def _compute_best_price(self):

for record in self:

record.best_price = max(record.offer_ids.mapped('price')) if record.offer_ids else 0

First check not needed

Mathilde411 · 2025-10-24T11:17:37Z

estate/models/estate_property.py

+    _check_expected_price = models.Constraint(
+         'CHECK(expected_price >= 0)', 'The expected price must be strictly positive.')
+
+    _check_selling_price = models.Constraint(
+         'CHECK(selling_price > 0)', 'The selling price must be positive.')
+
+    @api.constrains('selling_price', 'expected_price')
+    def _check_selling_price_expected_price(self):
+        for record in self:
+            if record.selling_price == 0:
+                continue
+            if float_compare(record.selling_price, 0.9 * record.expected_price, precision_digits=2) == -1:
+                raise UserError("The selling price must be at least 90% of the expected price.")


As per attribute ordering this needs to be higher

Mathilde411 · 2025-10-24T11:20:46Z

estate/models/estate_property_offer.py

+    def action_refuse(self):
+        for offer in self:
+            if offer.state != 'accepted':
+                offer.state = 'refused'
+            else:
+                raise UserError("An accepted offer cannot be refused.")
+        return True


Suggested change

def action_refuse(self):

for offer in self:

if offer.state != 'accepted':

offer.state = 'refused'

else:

raise UserError("An accepted offer cannot be refused.")

return True

def action_refuse(self):

if any(offer.state == 'accepted' for offer in self):

raise UserError("An accepted offer cannot be refused.")

self.state = 'refused'

return True

More concise

Mathilde411 · 2025-10-24T11:24:32Z

estate/__manifest__.py

+    ],
+    'installable': True,
+    'application': True,


Suggested change

],

'installable': True,

'application': True,

],

'application': True,

default value for installable is already True

[ADD] estate: create the app.

c2f4e01

Mathilde411 self-requested a review October 20, 2025 11:56

Mathilde411 changed the title ~~[ADD] estate: create the app.~~ HAZEI Onboarding Oct 20, 2025

Mathilde411 self-assigned this Oct 20, 2025

hazei-odoo added 2 commits October 20, 2025 15:04

[IMP] estate: fill the manifest

ae40a80

[IMP] estate: estate_property.py

2ebfca3

Mathilde411 requested changes Oct 20, 2025

View reviewed changes

hazei-odoo added 3 commits October 20, 2025 16:39

[IMP] estate: fix

c451738

[IMP] estate: fix2

e7f8afa

[IMP] estate: manifest

500f0fc

Mathilde411 self-requested a review October 21, 2025 11:18

Mathilde411 removed their assignment Oct 21, 2025

hazei-odoo added 3 commits October 21, 2025 15:42

[IMP] estate: CH6 basic view

c85b983

[IMP] estate: CH6 basic views

85903f1

[IMP] estate: CH6 basic views correction

26e9cb3

Mathilde411 requested changes Oct 22, 2025

View reviewed changes

hazei-odoo added 10 commits October 22, 2025 14:07

[IMP] estate: Ch7

dd4f93c

[IMP] estate: Ch7 fix

dca37b2

[IMP] estate: Ch7 fix1

5bee9d6

[IMP] estate: Ch8

fbe8107

[IMP] estate: Ch8 fix

b78a704

[IMP] estate: Ch8 fix1

cdbf0e2

[IMP] estate: Ch9

ae6c1ab

[IMP] estate: Ch9fix

4d8c35d

[IMP] estate: Ch9fix1

791378e

[IMP] estate: Ch 10

e66f5d0

alan-odoo reviewed Oct 23, 2025

View reviewed changes

hazei-odoo added 2 commits October 24, 2025 09:17

[FIX] estate: Fixing the code and xml

7e58d88

[FIX] estate: Fixing error with style

71ed848

Mathilde411 requested changes Oct 24, 2025

View reviewed changes

	date_availability = fields.Date(copy=False, readonly=True, default=date.today() + relativedelta(months=3))
	date_availability = fields.Date(copy=False, readonly=True, default=lambda self: date.today() + relativedelta(months=3))

	active = fields.Boolean(default=False)
	active = fields.Boolean()

	if (self.state != "Cancelled"):
	if self.state != "Cancelled":

	property.total_area = property.living_area + (property.garden_area or 0)
	property.total_area = property.living_area + property.garden_area

HAZEI Onboarding #1000

Are you sure you want to change the base?

HAZEI Onboarding #1000

Conversation

hazei-odoo commented Oct 20, 2025

Uh oh!

robodoo commented Oct 20, 2025

Uh oh!

Mathilde411 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mathilde411 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alan-odoo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Mathilde411 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants